Skip to content

Conversation

@o-200
Copy link
Contributor

@o-200 o-200 commented Aug 20, 2025

Changes proposed in this PR:

  • Fixed brakeman so it now runs
  • Updated brakeman action to activates only when changes are pushed to main branch. This ensures we are informed about vulnerabilities without blocking development
  • Set 'Tests - PostgreSQL' on push
  • Updated badges in README.md to show the status for main branch only

This pull request is the part of #3551

@aaronskiba aaronskiba self-requested a review August 20, 2025 15:28
Copy link
Contributor

@aaronskiba aaronskiba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @o-200, thank you for this contribution. Your #3551 issue addresses CI improvements we've very recently been discussing.

Along with this brakeman fix, triggering the Postgresql GH Action on pushes would also be very valuable. Would you like to include that CI improvement in this current PR? Or maybe you'd prefer it as a separate PR?

@o-200 o-200 changed the title Configure brakeman on github pages Improve CI configuration Aug 21, 2025
@o-200
Copy link
Contributor Author

o-200 commented Aug 21, 2025

Hi @o-200, thank you for this contribution. Your #3551 issue addresses CI improvements we've very recently been discussing.

Along with this brakeman fix, triggering the Postgresql GH Action on pushes would also be very valuable. Would you like to include that CI improvement in this current PR? Or maybe you'd prefer it as a separate PR?

Done, but what about 'Tests - MySQL' action? I'd prefer enabling it if the project uses two databases, but now it's action was disabled.

@aaronskiba
Copy link
Contributor

Hi @o-200, thank you for this contribution. Your #3551 issue addresses CI improvements we've very recently been discussing.
Along with this brakeman fix, triggering the Postgresql GH Action on pushes would also be very valuable. Would you like to include that CI improvement in this current PR? Or maybe you'd prefer it as a separate PR?

Done, but what about 'Tests - MySQL' action? I'd prefer enabling it if the project uses two databases, but now it's action was disabled.

Good question. It sounds like we will remove the MySQL support in the near future. (Just added a bit more about in the issue you created: #3551)

@aaronskiba aaronskiba changed the base branch from development to next-release/v5.0.2 August 21, 2025 14:28
@aaronskiba aaronskiba changed the base branch from next-release/v5.0.2 to development August 21, 2025 14:29
@aaronskiba
Copy link
Contributor

Thank you again for your work here @o-200. I had mentioned that these CI improvements were recently discussed. These discussions are related to a newly proposed release flow.

Sorry to complicate things with your contribution here. You have done a perfect job following our contributors guide, but the guide will have to be updated to reflect the new release flow. (I have described some of the needed updates here: #3554)

@andreadavanzo what would you propose here? I have just published the branch next-release/v5.0.2. Should the branch o-200:fix-brakeman be re-based off of that branch and should this PR also set it as the target?

@o-200
Copy link
Contributor Author

o-200 commented Sep 16, 2025

hey, @aaronskiba, i see that CI "Danger / danger (pull_request)" is crushing. It is a expected behavior?

@aaronskiba
Copy link
Contributor

hey, @aaronskiba, i see that CI "Danger / danger (pull_request)" is crushing. It is a expected behavior?

Yes, that is unfortuately a behaviour we've noticed with PRs from outside of the main repo. Sorry about that.

@aaronskiba
Copy link
Contributor

Hi @o-200, sorry for the huge delay with this one.

Given our newer release flow, rather than trying to merge PR, I went ahead and cherry-picked your changes and added them to our newest release. I hope that's alright.
Screenshot from 2025-10-08 09-56-05

Thank you very much for your contribution here, these are some very nice improvements.

@aaronskiba aaronskiba closed this Oct 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants